Source Okta: OAuth2.0 support - disabled #20877

lazebnyi · 2022-12-23T23:12:06Z

What

OAuth 2.0 temporarily disabled until full we get public account. - https://airbytehq.slack.com/archives/C02U9R3AF37/p1671731021620059

How

Deleted part of spec implementation.

lazebnyi · 2023-01-04T12:24:42Z

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3837955948
❌ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3837955948
🐛 https://gradle.com/s/2bva246hhtr4m

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs0] - AssertionError: as...
FAILED test_core.py::TestConnection::test_check[inputs1] - AssertionError: as...
FAILED test_core.py::TestConnection::test_check[inputs2] - AssertionError: as...
FAILED test_core.py::TestBasicRead::test_read[inputs0] - docker.errors.Contai...
FAILED test_core.py::TestBasicRead::test_read[inputs1] - docker.errors.Contai...
FAILED test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
============ 9 failed, 25 passed, 1 skipped, 31 warnings in 28.89s =============

lazebnyi · 2023-01-05T20:40:29Z

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3850217641
✅ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3850217641
Python tests coverage:

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_okta/utils.py              44      0   100%
source_okta/authenticator.py      14      0   100%
source_okta/__init__.py            2      0   100%
source_okta/source.py            194      7    96%
--------------------------------------------------
TOTAL                            254      7    97%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   56-63, 68-81, 244
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
================== 32 passed, 1 skipped in 217.57s (0:03:37) ===================

alafanechere · 2023-01-06T12:58:37Z

@lazebnyi could you please share more details about the motivation of this PR (I can't access the slack conversation). What will be the impact on existing cloud connections?

lazebnyi · 2023-01-06T13:08:10Z

@alafanechere During Okta certification to Beta we implement OAuth, but now we has problem with getting public application. So will be decided hide Okta OAuth flow from cloud (leave only credentials manual input) to avoid issue during authentication with common OAuth flow.

I also refactored the acceptance tests to high strictness for speed up future certification.

alafanechere · 2023-01-06T13:20:54Z

Thanks for the context. @lazebnyi does it mean that Okta OAuth is not working on cloud (because we don't have a public application) and its safe to disable it?

lazebnyi · 2023-01-06T14:37:06Z

@alafanechere
Okta OAuth has never worked on the cloud 😅
Yes, it's safe. With this PR, the user will be authenticated with client_id, client_secret and refresh_token or with an API token.

alafanechere

Thanks for the explanation and for setting high test strictness level!

lazebnyi · 2023-01-06T16:28:13Z

/publish connector=connectors/source-okta

🕑 Publishing the following connectors:
connectors/source-okta
https://github.com/airbytehq/airbyte/actions/runs/3856783659

Connector	Did it publish?	Were definitions generated?
connectors/source-okta	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

evantahler

Do we need to remove any oAuth secrets from Cloud? If users have previously used oAuth + Okta will the connector still try to use that method of connection from the config, even though the SEPC option is removed?

(edit) Looks like we never got the app authorized, so this is unlikely. Carry on!

lazebnyi · 2023-01-06T17:47:38Z

@evantahler we remove only part what get user authenticate with common OAuth flow (enter creds to provider and approve scopes). So, if user already authenticate using OAuth flow he has refresh_token and can refresh it with connector authentication logic (but it's not possible because we never has public application for our account, so OAuth authentication with UI never work)

We never had an public account for Okta, so we no need delete any oAuth secrets from Cloud.

pedroslopez · 2023-01-06T23:11:30Z

@evantahler / @lazebnyi - I do see we had configured global OAuth parameters in cloud prod for Okta. In the past we've seen leaving the OAuth parameters in place cause issues when removing OAuth support (e.g. PayPal Transactions pulling in the same data for all users), so whenever we decide to remove OAuth support we should also make sure to remove the global OAuth parameters that may have been set.

Since this has now been released to cloud, I have removed the global OAuth parameters for Okta from the Cloud prod db.

Note: I found one active OAuth connection (for non-internal/test workspaces) for Okta, but it's set to manual mode and the workspace is called "Okta Partner Account" so presumably we know about this one and are ok with this no longer working?

Okta Partner Account

* Disabled OAuth Okta * Updated acceptance tests * Updated PR number * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

Disabled OAuth Okta

8186f80

lazebnyi requested review from davydov-d and bazarnov December 23, 2022 23:12

octavia-squidington-iv added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/okta labels Dec 23, 2022

davydov-d approved these changes Dec 27, 2022

View reviewed changes

Merge branch 'master' into lazebnyi/disable-oauth-okta

d89d5b5

bazarnov approved these changes Jan 4, 2023

View reviewed changes

lazebnyi and others added 2 commits January 5, 2023 21:39

Updated acceptance tests

ab2c127

Merge branch 'master' into lazebnyi/disable-oauth-okta

2efea56

lazebnyi requested a review from pedroslopez January 5, 2023 20:50

lazebnyi added 2 commits January 5, 2023 21:58

Merge branch 'master' into lazebnyi/disable-oauth-okta

ea6df0f

Merge branch 'master' into lazebnyi/disable-oauth-okta

8e54594

lazebnyi requested review from alafanechere, evantahler and erohmensing January 6, 2023 12:45

alafanechere approved these changes Jan 6, 2023

View reviewed changes

lazebnyi removed request for evantahler, pedroslopez and erohmensing January 6, 2023 16:18

lazebnyi added 2 commits January 6, 2023 17:23

Updated PR number

345ff1b

Merged master to branch

abb747a

auto-bump connector version

a44df1a

octavia-squidington-iii temporarily deployed to more-secrets January 6, 2023 16:55 — with GitHub Actions Inactive

evantahler reviewed Jan 6, 2023

View reviewed changes

lazebnyi requested a review from evantahler January 6, 2023 17:48

evantahler approved these changes Jan 6, 2023

View reviewed changes

Merge branch 'master' into lazebnyi/disable-oauth-okta

5978960

lazebnyi temporarily deployed to more-secrets January 6, 2023 18:44 — with GitHub Actions Inactive

Merge branch 'master' into lazebnyi/disable-oauth-okta

63b7aea

lazebnyi temporarily deployed to more-secrets January 6, 2023 22:27 — with GitHub Actions Inactive

lazebnyi enabled auto-merge (squash) January 6, 2023 22:43

lazebnyi merged commit f9fc56c into master Jan 6, 2023

lazebnyi deleted the lazebnyi/disable-oauth-okta branch January 6, 2023 23:01

github-actions bot mentioned this pull request Jan 9, 2023

Bump helm chart version reference to 0.43.18 #21172

Closed

octavia-squidington-iii mentioned this pull request Jan 12, 2023

Bump Airbyte version from 0.40.27 to 0.40.28 #21359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Okta: OAuth2.0 support - disabled #20877

Source Okta: OAuth2.0 support - disabled #20877

lazebnyi commented Dec 23, 2022 •

edited

Loading

lazebnyi commented Jan 4, 2023 •

edited by github-actions bot

Loading

lazebnyi commented Jan 5, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 6, 2023

lazebnyi commented Jan 6, 2023

alafanechere commented Jan 6, 2023

lazebnyi commented Jan 6, 2023 •

edited

Loading

alafanechere left a comment

lazebnyi commented Jan 6, 2023 •

edited by github-actions bot

Loading

evantahler left a comment •

edited

Loading

lazebnyi commented Jan 6, 2023

pedroslopez commented Jan 6, 2023 •

edited

Loading

Source Okta: OAuth2.0 support - disabled #20877

Source Okta: OAuth2.0 support - disabled #20877

Conversation

lazebnyi commented Dec 23, 2022 • edited Loading

What

How

lazebnyi commented Jan 4, 2023 • edited by github-actions bot Loading

Build Failed

lazebnyi commented Jan 5, 2023 • edited by github-actions bot Loading

Build Passed

alafanechere commented Jan 6, 2023

lazebnyi commented Jan 6, 2023

alafanechere commented Jan 6, 2023

lazebnyi commented Jan 6, 2023 • edited Loading

alafanechere left a comment

Choose a reason for hiding this comment

lazebnyi commented Jan 6, 2023 • edited by github-actions bot Loading

evantahler left a comment • edited Loading

Choose a reason for hiding this comment

lazebnyi commented Jan 6, 2023

pedroslopez commented Jan 6, 2023 • edited Loading

lazebnyi commented Dec 23, 2022 •

edited

Loading

lazebnyi commented Jan 4, 2023 •

edited by github-actions bot

Loading

lazebnyi commented Jan 5, 2023 •

edited by github-actions bot

Loading

lazebnyi commented Jan 6, 2023 •

edited

Loading

lazebnyi commented Jan 6, 2023 •

edited by github-actions bot

Loading

evantahler left a comment •

edited

Loading

pedroslopez commented Jan 6, 2023 •

edited

Loading